DBI-798: Safer check on mirror validation with MySQL sources setting server_id by pfcoperez · Pull Request #4564 · PeerDB-io/peerdb

pfcoperez · 2026-07-10T18:16:23Z

It adds a check on the utilization of the same server_id in the source MySQL server.
This offer an extra layer of protection against collisions with other replication actors.

…server_id It adds a check on the utilization in the source DB of the server_id.

claude · 2026-07-10T18:29:40Z

Code review

Found 1 issue. Checked for bugs and CLAUDE.md compliance.

Bug: nil pointer dereference at flow/cmd/validate_mirror.go:199

mysqlConn.Conn() returns nil at this call site, causing a panic inside HasReplicaWithServerId.

MySqlConnector uses lazy connection initialization. NewMySqlConnector stores conn as nil and never calls connect(). The actual TCP connection is only established on-demand via Execute()/withRetries(), which internally call connect().

Conn() is just c.conn.Load() — a bare atomic load that returns nil until a connection has been established. Since connectors.GetAs only calls the constructor (no queries), Conn() returns nil. HasReplicaWithServerId then calls conn.Execute(SHOW REPLICAS) on nil, which panics.

Suggested fix: Change HasReplicaWithServerId to accept the MySqlConnector (or a context + an interface with Execute) so it goes through the lazy-connect path, or trigger connection establishment before calling Conn().

See: NewMySqlConnector, Conn(), HasReplicaWithServerId

jgao54 · 2026-07-10T18:30:18Z


+	// beyond other PeerDB mirrors (checked below), the pinned server_id must not be already registered
+	// as a replica on the source DB itself.
+	mysqlConn, mysqlClose, err := connectors.GetAs[*connmysql.MySqlConnector](ctx, cfg.Env, peer)


nit: can avoid an extra connect/disconnect by moving mysql_validation.HasReplicaWithServerId inside ValidateMirrorSource and pass in an already instantiated conn, although i understand that having this here put server_id validation check together. Your call.

codecov · 2026-07-10T18:36:43Z

❌ 4 Tests Failed:

Tests completed	Failed	Passed	Skipped
3041	4	3037	371

View the top 3 failed test(s) by shortest run time

github.com/PeerDB-io/peerdb/flow/e2e::TestApiMariaDB

Stack Traces | 0.01s run time

=== RUN   TestApiMariaDB
=== PAUSE TestApiMariaDB
=== CONT  TestApiMariaDB
--- FAIL: TestApiMariaDB (0.01s)
2026/07/10 18:27:35 INFO Received AWS credentials from peer for connector: ci x-peerdb-additional-metadata={Operation:FLOW_OPERATION_UNKNOWN}
2026/07/10 18:27:35 INFO Received AWS credentials from peer for connector: clickhouse x-peerdb-additional-metadata={Operation:FLOW_OPERATION_UNKNOWN}

github.com/PeerDB-io/peerdb/flow/e2e::TestApiMy

Stack Traces | 0.02s run time

=== RUN   TestApiMy
=== PAUSE TestApiMy
=== CONT  TestApiMy
--- FAIL: TestApiMy (0.02s)
2026/07/10 18:32:53 INFO Received AWS credentials from peer for connector: ci x-peerdb-additional-metadata={Operation:FLOW_OPERATION_UNKNOWN}
2026/07/10 18:32:53 INFO Received AWS credentials from peer for connector: clickhouse x-peerdb-additional-metadata={Operation:FLOW_OPERATION_UNKNOWN}

github.com/PeerDB-io/peerdb/flow/e2e::TestApiMy/TestValidateCDCMirror_ServerIDPeerReuse

Stack Traces | 0.05s run time

=== RUN   TestApiMy/TestValidateCDCMirror_ServerIDPeerReuse
=== PAUSE TestApiMy/TestValidateCDCMirror_ServerIDPeerReuse
=== CONT  TestApiMy/TestValidateCDCMirror_ServerIDPeerReuse
2026/07/10 18:31:25 INFO Received AWS credentials from peer for connector: ci x-peerdb-additional-metadata={Operation:FLOW_OPERATION_UNKNOWN}
2026/07/10 18:31:25 INFO Received AWS credentials from peer for connector: clickhouse x-peerdb-additional-metadata={Operation:FLOW_OPERATION_UNKNOWN}
    api_test.go:381: 
        	Error Trace:	.../flow/e2e/api_test.go:381
        	Error:      	Received unexpected error:
        	            	rpc error: code = Internal desc = internal server error
        	Test:       	TestApiMy/TestValidateCDCMirror_ServerIDPeerReuse
    api_test.go:51: begin tearing down postgres schema api_evlgjbe7
2026/07/10 18:31:25 INFO fetched schema x-peerdb-additional-metadata={Operation:FLOW_OPERATION_UNKNOWN} table=e2e_test_mych_yawdnspt.test_bit
--- FAIL: TestApiMy/TestValidateCDCMirror_ServerIDPeerReuse (0.05s)

github.com/PeerDB-io/peerdb/flow/e2e::TestApiMariaDB/TestValidateCDCMirror_ServerIDPeerReuse

Stack Traces | 0.06s run time

=== RUN   TestApiMariaDB/TestValidateCDCMirror_ServerIDPeerReuse
=== PAUSE TestApiMariaDB/TestValidateCDCMirror_ServerIDPeerReuse
=== CONT  TestApiMariaDB/TestValidateCDCMirror_ServerIDPeerReuse
2026/07/10 18:26:00 INFO Received AWS credentials from peer for connector: ci x-peerdb-additional-metadata={Operation:FLOW_OPERATION_UNKNOWN}
2026/07/10 18:26:00 INFO Received AWS credentials from peer for connector: clickhouse x-peerdb-additional-metadata={Operation:FLOW_OPERATION_UNKNOWN}
    api_test.go:381: 
        	Error Trace:	.../flow/e2e/api_test.go:381
        	Error:      	Received unexpected error:
        	            	rpc error: code = Internal desc = internal server error
        	Test:       	TestApiMariaDB/TestValidateCDCMirror_ServerIDPeerReuse
    api_test.go:51: begin tearing down postgres schema api_umm4t365
--- FAIL: TestApiMariaDB/TestValidateCDCMirror_ServerIDPeerReuse (0.06s)

To view more test analytics, go to the Test Analytics Dashboard
_{📋 Got 3 mins? Take this short survey to help us improve Test Analytics.}

github-actions · 2026-07-10T18:40:42Z

❌ Test Failure

Analysis: The PR's own new test TestValidateCDCMirror_ServerIDPeerReuse fails deterministically across all three MySQL matrix jobs with an "internal server error" RPC response, indicating a real bug in the new server_id validation code rather than a flaky/timing/network issue.
Confidence: 0.94

⚠️ This appears to be a real bug - manual intervention needed

View workflow run

jgao54 · 2026-07-10T22:48:02Z

(pending integrations test fixes)

DBI-798: Safer check on mirror validation with MySQL sources setting …

19c30f3

…server_id It adds a check on the utilization in the source DB of the server_id.

pfcoperez requested a review from a team as a code owner July 10, 2026 18:16

jgao54 approved these changes Jul 10, 2026

View reviewed changes

jgao54 reviewed Jul 10, 2026

View reviewed changes

Reorder code

5b0ffb0

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

DBI-798: Safer check on mirror validation with MySQL sources setting server_id#4564

DBI-798: Safer check on mirror validation with MySQL sources setting server_id#4564
pfcoperez wants to merge 2 commits into
mainfrom
DBI-798/connectors/mysql/improved-server-id-check

pfcoperez commented Jul 10, 2026

Uh oh!

claude Bot commented Jul 10, 2026

Uh oh!

jgao54 Jul 10, 2026

Uh oh!

codecov Bot commented Jul 10, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Jul 10, 2026

Uh oh!

jgao54 commented Jul 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

pfcoperez commented Jul 10, 2026

Uh oh!

claude Bot commented Jul 10, 2026

Code review

Uh oh!

jgao54 Jul 10, 2026

Choose a reason for hiding this comment

Uh oh!

codecov Bot commented Jul 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

❌ 4 Tests Failed:

Uh oh!

github-actions Bot commented Jul 10, 2026

❌ Test Failure

Uh oh!

jgao54 commented Jul 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codecov Bot commented Jul 10, 2026 •

edited

Loading